-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add a browser lib to produce sa11y minified JS bundle #32
Conversation
# Conflicts: # package.json
# Conflicts: # package.json # yarn.lock
based on recommendations from https://tools.ietf.org/id/draft-knodel-terminology-00.html
that has been moved to wdio
use explicit sa11y version for checking
minor doc fixes
@@ -0,0 +1,28 @@ | |||
# `@sa11y/browser-lib` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if browser-lib
is an apt name for the library that produces sa11y.min.js.
Suggestions welcome!
due to cicleci perm errors related to third party codecov orb
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
==========================================
Coverage ? 100.00%
==========================================
Files ? 14
Lines ? 112
Branches ? 11
==========================================
Hits ? 112
Misses ? 0
Partials ? 0 |
|
||
Provides a minified version of selected `@sa11y` libraries to be injected into a browser (using webdriver) and executed from integration testing workflows. This allows for reuse of the `@sa11y` libraries across unit and integration testing workflows. | ||
|
||
Code in this package should be limited only to wrappers required to facilitate execution in browser environment. All primary code should be added to `@sa11y` libraries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little unclear when you would use this package vs. the wdio package. I see the example usage below looks like it's even wdio
sample code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is to be used in testing workflows without webdriverIO e.g. Java. Added clarification to doc. Please let me know if that answers your question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok got it. That makes sense, but I still find it a little confusing that the sample below is in Javascript if the main target is Java. It'd be nice to see a more complete example authored in Java, even if the test cases in this project are still Javascript.
If I was a consumer trying to integrate this into my Java test suite, it would take some exploration still to figure out how exactly to use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, was thinking the same. Added Java example.
package.json
Outdated
@@ -17,8 +17,9 @@ | |||
], | |||
"scripts": { | |||
"build": "tsc --build", | |||
"build:ci": "yarn install --frozen-lockfile && yarn build", | |||
"build:clean": "yarn build --clean && rimraf packages/**/dist && yarn install && yarn build", | |||
"build:minjs": "yarn workspace @sa11y/browser-lib build", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect this to run during the normal build
, instead of only build:ci
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Fixed. Was trying to hold on to tsc --build
being used only once and reusing yarn build
.
resolve(), | ||
commonjs(), | ||
typescript({ tsconfigOverride: { compilerOptions: { module: 'es2015' } } }), | ||
terser(), // Note: Comment to get un-minified file for debugging etc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also produce an un-minified sa11y.js
in addition to the minified sa11y.min.js
. Then the consumer would choose which version they wanted to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
packages/browser-lib/src/index.ts
Outdated
export { sortViolations } from '@sa11y/format'; | ||
export { assertAccessible } from '@sa11y/assert'; | ||
|
||
export const nameSpace = 'sa11y'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is usually "namespace" (all lowercase).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
||
Provides a minified version of selected `@sa11y` libraries to be injected into a browser (using webdriver) and executed from integration testing workflows. This allows for reuse of the `@sa11y` libraries across unit and integration testing workflows. | ||
|
||
Code in this package should be limited only to wrappers required to facilitate execution in browser environment. All primary code should be added to `@sa11y` libraries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok got it. That makes sense, but I still find it a little confusing that the sample below is in Javascript if the main target is Java. It'd be nice to see a more complete example authored in Java, even if the test cases in this project are still Javascript.
If I was a consumer trying to integrate this into my Java test suite, it would take some exploration still to figure out how exactly to use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @trevor-bliss
Addressed your comments. Please review.
sa11y.min.js
that wraps overaxe-core
librarysa11y.min.js
would be injected into the browser, from webdriver tests e.g., instead ofaxe-core
JS library to provide the same set of Sa11y APIs across different testing workflowssa11y.min.js
would still expose theaxe-core
library same asaxe.min.js
so the existing APIs relying onaxe.min.js
would continue to work without having to rewrite everything to make use of the Sa11y APIssa11y.min.js
TODO
Post merge